fix(structures): fix fetchMessages pagination and dropped in-memory msgs#201711
Closed
themazim wants to merge 3 commits intowwebjs:mainfrom
Closed
fix(structures): fix fetchMessages pagination and dropped in-memory msgs#201711themazim wants to merge 3 commits intowwebjs:mainfrom
themazim wants to merge 3 commits intowwebjs:mainfrom
Conversation
Builds on wwebjs#201705 by JuniorRibas. Replaces WAWebChatLoadMessages.loadEarlierMsgs with WAWebDBMessageFindLocal.msgFindBefore when paginating messages until reaching searchOptions.limit in both Chat and Channel, resolving models through MsgStore when the returned entries are not yet serialized. Follow-up fix: in the !fromMeFilter && finite branch of both Chat.fetchMessages and Channel.fetchMessages, include the already-loaded in-memory msgs when constructing merged. The original PR discarded them, causing chats whose requested window consists mostly of recent in-memory messages (with nothing older resolvable via msgFindBefore) to return only the anchor message. dedupeByMsgId keeps the result unique.
When msgFindBefore returns status 404 (anchor message not in local DB), the fast-path in Chat/Channel.fetchMessages was discarding the already-filtered in-memory msgs. Mirror the !anchorSerialized fallback directly above: sort by t and slice to the limit, keeping in-memory messages rather than returning an empty array. Also adds short inline comments to the non-obvious decisions introduced by wwebjs#201705: the msgFindByDirection fallback, the in-memory msgs merge, and the new 404 fallback — aimed at making upstream review easier.
themazim
changed the title
3 tasks
3 tasks
|
I had the same issue mentioned here #201706 and followed your solution. Can confirm it's working as expected and I'm able to fetch messages again |
|
It's working perfectly for me. Thanks |
Member
|
What is the difference between #201705 and that one? |
Contributor
Author
yes, and this: The initial PR (#201705) seems to be highly over engineered regarding the initial issue., now that I have dug way deeper into this. Simple method signature change would probably fix this as well. |
Member
|
If the difference is only that, make your suggestions in the original PR |
Contributor
Author
as expected, the signature just changed. #201713 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #201705. Builds on that refactor and fixes two cases where
Chat.fetchMessages/Channel.fetchMessagescould return fewer messages than requested — in particular, an empty array or a single anchor message where the caller expected up tolimitmessages.Changes (both
src/structures/Chat.jsandsrc/structures/Channel.js, in the!fromMeFilter && finitefast-path introduced by #201705):Include in-memory
msgswhen buildingmerged.loadedonly contains whatmsgFindBeforereturned from the local IndexedDB message table. Messages that are in the in-memoryMsgCollectionbut not yet persisted (or not returned by the range query) were being dropped. Appending...msgsintomergedpreserves them; the existingdedupeByMsgIdstep guarantees no duplicates.Symptom before the fix: chats whose requested window consisted mostly of recent in-memory messages (with nothing older resolvable via
msgFindBefore) returned only the anchor message — reproduced and confirmed in the refactor(structures): optimize message loading with msgFindBefore #201705 discussion thread.Preserve
msgsonmsgFindBeforestatus: 404.When the anchor is not present in the local DB (
NoAnchorMessageError→status: 404), the fast-path previously setmsgs = [], throwing away the already-filtered in-memory messages. It now mirrors the!anchorSerializedfallback immediately above: sort bytand slice tolimit. Rare in practice (lastReceivedKeyis almost always present in the DB), but consistent with how the "cannot run the DB query" case is handled a few lines above.No public API or type changes.
Related Issue(s)
Builds on #201705.
Testing Summary
Test Details
npx eslint src/structures/Chat.js src/structures/Channel.js— clean.ChatandChannel:chat.fetchMessages({ limit: 50 })on a chat with a large stored history → returns ~50 messages (regression guard for the single-message bug).chat.fetchMessages({ limit: 50 })on a chat whose history mostly lives in the in-memory collection → now returns the in-memory messages instead of one or zero.channel.fetchMessages({ limit: 50 }).Type of Change
Checklist
npm test). (integration suite requiresWWEBJS_TEST_REMOTE_ID; eslint on the touched files is clean)index.d.ts) have been updated if necessary. (no public API changes)example.js) / documentation have been updated if applicable. (not applicable)